Skip to content

fix(sessions): redact connection-URI password in unsupported-URI errors - #602

Open
herdiyana256 wants to merge 3 commits into
google:mainfrom
herdiyana256:fix/redact-connection-uri-password-in-errors
Open

fix(sessions): redact connection-URI password in unsupported-URI errors#602
herdiyana256 wants to merge 3 commits into
google:mainfrom
herdiyana256:fix/redact-connection-uri-password-in-errors

Conversation

@herdiyana256

Copy link
Copy Markdown
Contributor

getSessionServiceFromUri (sessions/registry.ts) and getConnectionOptionsFromUri (sessions/db/operations.ts) interpolate the raw connection URI into their "Unsupported ... URI" error messages. A connection URI such as postgres://user:password@host/db carries the password in its userinfo component, so an unsupported or mistyped scheme surfaces the password verbatim in the thrown Error, which typically propagates to application logs and error-tracking services — a different trust boundary from whoever provisioned the connection string.

This adds a redactUriPassword helper that masks the userinfo password while keeping the rest of the URI intact for debugging (mirroring the semantics of Go net/url.URL.Redacted, and the render_as_string(hide_password=True) redaction already used in the adk-python DatabaseSessionService), and applies it at both error sites. Unparseable inputs fall back to returning only the scheme prefix so a credential in a non-URL string is not leaked either. Adds unit and regression tests covering both call sites.

getSessionServiceFromUri and getConnectionOptionsFromUri interpolated the raw
connection URI into their "Unsupported ... URI" error messages. A URI such as
postgres://user:password@host/db embeds the password in its userinfo, so an
unsupported or mistyped scheme surfaced the password verbatim in a thrown Error,
which typically reaches application logs and error-tracking services. Add a
redactUriPassword helper that masks the userinfo password (keeping the rest of
the URI for debugging, like Go's net/url.URL.Redacted) and use it at both sites.
secretlint's database-connection-string rule flags the literal
postgres:// URI in the test as a real credential because "s3cr3t"
isn't on its placeholder allowlist. Use "pass", matching the
convention already used by the sibling operations_test.ts fixtures.
@herdiyana256

herdiyana256 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Pushed a follow-up commit to fix the failing run-tests checks — secretlint's database-connection-string rule was flagging the test's literal postgres://user:s3cr3t@db.host:5432/mydb as a real credential (s3cr3t isn't on its placeholder allowlist, unlike pass/password used elsewhere in the test suite). Swapped it to pass to match the existing convention; verified secretlint and the full unit suite pass locally.

@kalenkevich the 4 workflow runs on the new commit are stuck in "awaiting approval" since this is a fork PR could you approve them when you get a chance so CI can finish?

format:check only surfaced this now that the earlier secretlint
failure stopped masking it — the ternary in the catch branch wasn't
prettier-formatted.
@herdiyana256

Copy link
Copy Markdown
Contributor Author

Pushed another follow-up commit — after the secretlint fix let run-tests proceed further, format:check (prettier) then failed on core/src/utils/redact_uri.ts (this had been masked by the earlier secretlint failure, which ran first in the same job). Ran prettier --write on that file; npx prettier --check "**/*.ts" is now clean repo-wide.

@kalenkevich could you approve the pending workflow runs on this commit too? Should be green after this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants